fix(interior sun): volumetric shadows compatibility by Dlizzio · Pull Request #2319 · community-shaders/skyrim-community-shaders

Dlizzio · 2026-05-11T15:45:29Z

Adds compatibility for interior sun and volumetric shadows.

Summary by CodeRabbit

New Features
- Interior sun reports active state to rendering for more consistent interior lighting
Improvements
- Directional shadow evaluation refined to respect current scene state (improves correctness)
- Volumetric shadow processing is skipped when directional shadows are disabled to reduce work
Chores
- Bumped Interior Sun shader feature to v1-0-1
- Bumped Volumetric Shadows shader feature to v2-0-1

github-actions · 2026-05-11T15:46:50Z

No actionable suggestions for changed features.

coderabbitai · 2026-05-11T15:48:28Z

📝 Walkthrough

Walkthrough

SharedData and State gain a HasDirectionalShadows field populated from State::HasDirectionalShadows(). InteriorSun exposes IsActiveInteriorSun(). Shaders add ShadowSampling::HasDirectionalShadows() and use it to gate volumetric/directional shadow computation. VolumetricShadows centralizes SRV binding and early-exits when directional shadows are disabled. Feature INI versions incremented.

Changes

Interior Sun Directional Shadow Support

Layer / File(s)	Summary
Constant Buffer & State `src/State.h`, `src/State.cpp`, `package/Shaders/Common/SharedData.hlsli`	Adds `HasDirectionalShadows` to CPU and shader shared-data layouts; State now includes header for InteriorSun and populates the field via `State::HasDirectionalShadows()`.
Interior Sun Feature Query `src/Features/InteriorSun.h`	Adds `IsActiveInteriorSun() const` to report whether the interior-sun feature is loaded and currently providing sun in interiors.
Shader Predicate `package/Shaders/Common/ShadowSampling.hlsli`	Adds `ShadowSampling::HasDirectionalShadows()` and gates `Get3DFilteredShadow()` / `GetLightingShadow()` to skip volumetric/directional computations when false.
Volumetric Shadows Binding & Early Exit `src/Features/VolumetricShadows.h`, `src/Features/VolumetricShadows.cpp`	Adds `kSharedShadowMapShaderSlot`, introduces `SetSharedShadowMapSRV()` helper, centralizes SRV selection/binding, and early-exits `CopyShadowLightData()` when directional shadows are disabled.
Shader Lighting Integration `package/Shaders/Effect.hlsl`, `package/Shaders/Lighting.hlsl`	Replaces `!SharedData::InInterior` checks with `ShadowSampling::HasDirectionalShadows()` in directional shadow evaluation paths.
Feature Version Updates `features/Interior Sun/Shaders/Features/InteriorSun.ini`, `features/Volumetric Shadows/Shaders/Features/VolumetricShadows.ini`	Bumps InteriorSun to `1-0-1` and VolumetricShadows to `2-0-1`.

Sequence Diagram(s)

(omitted — changes are localized and straightforward sequential interactions are already represented in the layer table)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

community-shaders/skyrim-community-shaders#2294: Modifies Effect.hlsl and interior handling; related to shader lighting gating changes.
community-shaders/skyrim-community-shaders#1685: Changes InteriorSun feature handling of interior-with-sun flag; related API usage.
community-shaders/skyrim-community-shaders#2182: Touches volumetric shadow SRV binding and shadow-sampling logic; related to SRV and volumetric changes.

Suggested reviewers

doodlum
alandtse

Poem

🐰 Inside the halls where sunlight creeps,
A tiny flag decides who sleeps—
Shaders listen, gates swing wide,
Volumetric maps now choose the side. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding compatibility between interior sun and volumetric shadows features, which is reflected throughout all modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (2)

features/Interior Sun/Shaders/Features/InteriorSun.ini (1)
2-2: ⚡ Quick win

Add an issue-closing reference in the PR description.

Since this PR is a compatibility fix, please add a keyword reference like Fixes #... or Closes #... for traceability.

As per coding guidelines, "Issue References (if PR fixes bugs or implements features): Suggest adding appropriate GitHub keywords: 'Fixes #123' or 'Closes #123' for bug fixes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/Interior` Sun/Shaders/Features/InteriorSun.ini at line 2, Update the
PR description to include a GitHub issue-closing keyword (e.g., "Fixes `#123`" or
"Closes `#123`") referencing the relevant issue for this compatibility fix; ensure
the chosen keyword appears in the PR body so merging will close the issue
automatically and add traceability for the change to Features/Interior Sun (the
change associated with Version = 1-0-1).
package/Shaders/Common/ShadowSampling.hlsli (1)
108-119: 💤 Low value

Minor: Unreachable code when VOLUMETRIC_SHADOWS is not defined.

When VOLUMETRIC_SHADOWS is not defined, the #else block on line 116 returns, making line 119 unreachable. The compiler will optimize this away, but the structure could be cleaner.
♻️ Suggested restructure
 `#if` defined(VOLUMETRIC_SHADOWS)
 		if (HasDirectionalShadows()) {
 			float vsmSurfaceShadow;
 			float shadow = VolumetricShadows::GetVSMShadow3D(startPosition, endPosition, noise, sampleCount, eyeIndex, vsmSurfaceShadow);
 			surfaceShadow *= vsmSurfaceShadow;
 			return worldShadow * shadow;
 		}
-#else
-		return worldShadow;
 `#endif`
 
 		return worldShadow;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package/Shaders/Common/ShadowSampling.hlsli` around lines 108 - 119, The
else-branch returning worldShadow makes the subsequent "return worldShadow;"
unreachable when VOLUMETRIC_SHADOWS is not defined; remove the redundant final
return or restructure the preprocessor blocks so there is exactly one return
path. Locate the block using HasDirectionalShadows(),
VolumetricShadows::GetVSMShadow3D, vsmSurfaceShadow, surfaceShadow and
worldShadow and either move the final "return worldShadow" into the `#else` (and
delete the duplicate after the `#endif`) or remove the `#else-return` and keep the
single return after the `#endif` so the code is clean and unambiguous.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@features/Interior` Sun/Shaders/Features/InteriorSun.ini:
- Line 2: Update the PR description to include a GitHub issue-closing keyword
(e.g., "Fixes `#123`" or "Closes `#123`") referencing the relevant issue for this
compatibility fix; ensure the chosen keyword appears in the PR body so merging
will close the issue automatically and add traceability for the change to
Features/Interior Sun (the change associated with Version = 1-0-1).

In `@package/Shaders/Common/ShadowSampling.hlsli`:
- Around line 108-119: The else-branch returning worldShadow makes the
subsequent "return worldShadow;" unreachable when VOLUMETRIC_SHADOWS is not
defined; remove the redundant final return or restructure the preprocessor
blocks so there is exactly one return path. Locate the block using
HasDirectionalShadows(), VolumetricShadows::GetVSMShadow3D, vsmSurfaceShadow,
surfaceShadow and worldShadow and either move the final "return worldShadow"
into the `#else` (and delete the duplicate after the `#endif`) or remove the
`#else-return` and keep the single return after the `#endif` so the code is clean
and unambiguous.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0eb62b20-79a2-4cd6-87ab-d46763865c7b

📥 Commits

Reviewing files that changed from the base of the PR and between 5a59442 and b5929d1.

📒 Files selected for processing (11)

features/Interior Sun/Shaders/Features/InteriorSun.ini
features/Volumetric Shadows/Shaders/Features/VolumetricShadows.ini
package/Shaders/Common/ShadowSampling.hlsli
package/Shaders/Common/SharedData.hlsli
package/Shaders/Effect.hlsl
package/Shaders/Lighting.hlsl
src/Features/InteriorSun.h
src/Features/VolumetricShadows.cpp
src/Features/VolumetricShadows.h
src/State.cpp
src/State.h

alandtse

Is this violating the feature boundary? We shouldn't have direct dependencies on features unless it's core.

github-actions · 2026-05-11T16:19:34Z

✅ A pre-release build is available for this PR:
Download

Dlizzio · 2026-05-11T16:30:17Z

Is this violating the feature boundary? We shouldn't have direct dependencies on features unless it's core.

They are both in core but I changed it anyways.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/State.cpp`:
- Around line 1087-1090: HasDirectionalShadows() can be true while no shadow SRV
is actually bound and the shader calls missing VolumetricShadows functions; fix
by making the CPU-side predicate require a valid SRV before returning true and
by either providing stub implementations or removing/guarding calls to the
missing functions in the shader. Concretely: in State::HasDirectionalShadows()
check the shadow SRV (the same resource checked in CopyShadowLightData /
PSGetShaderResources slot 4) and only return true when a non-null SRV is
available; in CopyShadowLightData ensure you never bind a null to shader slot 18
(bind a default/empty SRV or skip binding and clear the directional-shadow
flag); and update ShadowSampling.hlsli to either implement
VolumetricShadows::GetVSMShadow2D/GetVSMShadow3D or wrap their calls behind a
compile-time or runtime guard so the shader won’t reference undefined functions
when the SRV is absent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1762c1a4-1e98-473b-9fbd-3f92d5fb34c0

📥 Commits

Reviewing files that changed from the base of the PR and between b5929d1 and 32cfe56.

📒 Files selected for processing (5)

package/Shaders/Common/ShadowSampling.hlsli
package/Shaders/Common/SharedData.hlsli
src/Features/VolumetricShadows.cpp
src/State.cpp
src/State.h

alandtse · 2026-05-12T05:04:57Z

@coderabbitai please confirm all includes are necessary and weren't made extraneous after some of the later commits to preserve the feature boundary.

coderabbitai · 2026-05-12T05:06:02Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me inspect the changed files to verify all includes.

🏁 Script executed:

#!/bin/bash
echo "=== State.cpp includes and usage ==="
grep -n '#include' src/State.cpp
echo ""
echo "=== State.cpp HasDirectionalShadows usage ==="
grep -n 'HasDirectionalShadows\|InteriorSun\|interiorSun' src/State.cpp

echo ""
echo "=== VolumetricShadows.cpp includes ==="
grep -n '#include' src/Features/VolumetricShadows.cpp
echo ""
echo "=== VolumetricShadows.cpp InteriorSun usage ==="
grep -n 'InteriorSun\|interiorSun' src/Features/VolumetricShadows.cpp

echo ""
echo "=== VolumetricShadows.h includes ==="
grep -n '#include' src/Features/VolumetricShadows.h
echo ""
echo "=== VolumetricShadows.h InteriorSun usage ==="
grep -n 'InteriorSun\|interiorSun' src/Features/VolumetricShadows.h

echo ""
echo "=== InteriorSun.h includes ==="
grep -n '#include' src/Features/InteriorSun.h

echo ""
echo "=== ShadowSampling.hlsli includes ==="
grep -n '#include' package/Shaders/Common/ShadowSampling.hlsli

echo ""
echo "=== SharedData.hlsli includes ==="
grep -n '#include' package/Shaders/Common/SharedData.hlsli

echo ""
echo "=== Effect.hlsl relevant usage ==="
grep -n 'HasDirectionalShadows\|InInterior\|InteriorSun' package/Shaders/Effect.hlsl

echo ""
echo "=== Lighting.hlsl relevant usage ==="
grep -n 'HasDirectionalShadows\|InInterior\|InteriorSun' package/Shaders/Lighting.hlsl

Length of output: 1679

🏁 Script executed:

#!/bin/bash
echo "=== Finding source files ==="
fd -t f '(State\.cpp|State\.h|VolumetricShadows\.cpp|VolumetricShadows\.h|InteriorSun\.h|InteriorSun\.cpp|ShadowSampling\.hlsli|SharedData\.hlsli)'

echo ""
echo "=== PR diff to check new includes ==="
git diff origin/dev...HEAD -- '*.cpp' '*.h' | grep -E '^(\+\+\+|---|\+#include|-#include)'

Length of output: 405

🏁 Script executed:

#!/bin/bash
echo "=== VolumetricShadows.cpp diff ==="
git diff origin/dev...HEAD -- src/Features/VolumetricShadows.cpp

echo ""
echo "=== State.cpp diff ==="
git diff origin/dev...HEAD -- src/State.cpp

Length of output: 2346

@alandtse: All includes have been verified. Here's the summary:

Only one new #include was added in this entire PR:

#include "Features/InteriorSun.h" in src/State.cpp

This include is necessary and not extraneous — it is actively consumed by the new State::HasDirectionalShadows() method:

bool State::HasDirectionalShadows() const
{
    return !Util::IsInterior() || globals::features::interiorSun.IsActiveInteriorSun();
}

Feature boundary is preserved. The cross-feature coordination is routed correctly:

VolumetricShadows.cpp does not include InteriorSun.h. It calls globals::state->HasDirectionalShadows() — going through State (core infrastructure), not directly to InteriorSun.
State.cpp is the appropriate place for this include; it already includes several other feature headers (CloudShadows.h, HDRDisplay.h, PerformanceOverlay.h, TerrainBlending.h, TerrainHelper.h) as part of its role as the central shared-data coordinator.

No includes were added to VolumetricShadows.h, VolumetricShadows.cpp (beyond what was pre-existing), InteriorSun.h, or State.h. Everything looks clean.

(cherry picked from commit 9353367)

Two regressions converged to leave the directional sun unshadowed and either fully bright or fully dark depending on cell type: 1. SCM's Hook_DisableColorMask in ShadowCasterManager.cpp was patching out a 5-byte CALL inside Main::RenderShadowmasks (100422/107140) under the assumption that the call targeted a "DrawColorMask" routine. Ghidra-verified on SE 1.5.97 (+0x90 -> 0x1412e3b80) and SkyrimVR (+0x9E -> 0x141323740): the call is to RenderShadowLightsWithUtilityShader (100423/107141) -- the function that emits every screen-space shadow mask draw. The hook therefore killed the entire kSHADOW_MASK RT every frame; the "Shadowmasks" engine marker was empty in RenderDoc. Replaced with a detour on 100423/107141 that writes a null sentinel into shadowLightsAccum at the 4-slice boundary, runs vanilla, then restores. The engine's iteration terminates cleanly at the sentinel without OOB-reading the 4-entry DAT_141861380 blend-mode table for extended-mode slots, while slots 0..3 (sun cascades + first vanilla point lights) get their mask draws as before. 2. Lighting.hlsl and Particle.hlsl gated the LLF directional shadow path on a bare !SharedData::InInterior check, so Interior Sun cells (where the engine has valid directional data) couldn't reach the cascade + engine-mask sampling. Mirrors community-shaders#2319's fix on the VOLUMETRIC_SHADOWS branch by switching to ShadowSampling::HasDirectionalShadows() which evaluates !IsInterior() || InteriorSun::IsActive. Stale comments referencing Hook_DisableColorMask updated in LightLimitFix.cpp and ShadowRenderer.cpp; the defensive code that reads the SCM-assigned slot rather than shadowmapDescriptors[0] is still correct, only its rationale shifted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two regressions converged to leave the directional sun unshadowed and either fully bright or fully dark depending on cell type: 1. SCM's Hook_DisableColorMask in ShadowCasterManager.cpp was patching out a 5-byte CALL inside Main::RenderShadowmasks (100422/107140) under the assumption that the call targeted a "DrawColorMask" routine. Ghidra-verified on SE 1.5.97 (+0x90 -> 0x1412e3b80) and SkyrimVR (+0x9E -> 0x141323740): the call is to RenderShadowLightsWithUtilityShader (100423/107141) -- the function that emits every screen-space shadow mask draw. The hook therefore killed the entire kSHADOW_MASK RT every frame; the "Shadowmasks" engine marker was empty in RenderDoc. Replaced with a detour on 100423/107141 that writes a null sentinel into shadowLightsAccum at the 4-slice boundary, runs vanilla, then restores. The engine's iteration terminates cleanly at the sentinel without OOB-reading the 4-entry DAT_141861380 blend-mode table for extended-mode slots, while slots 0..3 (sun cascades + first vanilla point lights) get their mask draws as before. 2. Lighting.hlsl and Particle.hlsl gated the LLF directional shadow path on a bare !SharedData::InInterior check, so Interior Sun cells (where the engine has valid directional data) couldn't reach the cascade + engine-mask sampling. Mirrors #2319's fix on the VOLUMETRIC_SHADOWS branch by switching to ShadowSampling::HasDirectionalShadows() which evaluates !IsInterior() || InteriorSun::IsActive. Stale comments referencing Hook_DisableColorMask updated in LightLimitFix.cpp and ShadowRenderer.cpp; the defensive code that reads the SCM-assigned slot rather than shadowmapDescriptors[0] is still correct, only its rationale shifted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…s#2319)

init

4c57139

version bump

b5929d1

coderabbitai Bot reviewed May 11, 2026

View reviewed changes

alandtse reviewed May 11, 2026

View reviewed changes

Comment thread package/Shaders/Common/ShadowSampling.hlsli Outdated

rev

32cfe56

coderabbitai Bot reviewed May 11, 2026

View reviewed changes

Comment thread src/State.cpp

alandtse approved these changes May 12, 2026

View reviewed changes

alandtse merged commit 9353367 into community-shaders:dev May 12, 2026
13 checks passed

alandtse pushed a commit that referenced this pull request May 12, 2026

fix(interior sun): volumetric shadows compatibility (#2319)

924a6ed

(cherry picked from commit 9353367)

SkrubbySkrubInAShrub pushed a commit that referenced this pull request May 14, 2026

fix(interior sun): volumetric shadows compatibility (#2319)

5fe0d8b

(cherry picked from commit 9353367)

coderabbitai Bot mentioned this pull request May 22, 2026

feat(volumetric-shadows)!: replace VSM with PCF froxel grid #2400

Closed

7 tasks

alandtse mentioned this pull request May 28, 2026

chore: drop issue/PR refs from code comments alandtse/open-shaders#54

Merged

IgorAlanAlbuquerque pushed a commit to IgorAlanAlbuquerque/skyrim-community-shaders that referenced this pull request May 29, 2026

fix(interior sun): volumetric shadows compatibility (community-shader…

c1626ba

…s#2319)

Dlizzio deleted the interior-sun-volumetric-shadows branch June 3, 2026 14:08

This was referenced Jun 5, 2026

feat(skysync): rework shadow fader, expose sun/moon to shared data #2408

Merged

feat(volumetric-shadows): EVSM shadow mapping with global depth normalization #2493

Merged

feat(skylighting): add per-probe shadow skylighting #2498

Draft

Conversation

Dlizzio commented May 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

github-actions Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

alandtse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dlizzio commented May 11, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alandtse commented May 12, 2026

Uh oh!

coderabbitai Bot commented May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Dlizzio commented May 11, 2026 •

edited by coderabbitai Bot

Loading

github-actions Bot commented May 11, 2026 •

edited

Loading

coderabbitai Bot commented May 11, 2026 •

edited

Loading

github-actions Bot commented May 11, 2026 •

edited

Loading